Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return http.StatusMethodNotAllowed for request method mismatch #25

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nek023
Copy link

@nek023 nek023 commented Oct 19, 2024

Overview

I fixed OapiRequestValidator to return http.StatusMethodNotAllowed instead of http.StatusNotFound when the request method does not match the routing definition.

Details

When the request method does not match the routing definition, the router returns ErrMethodNotAllowed.
However, in the current implementation of OapiRequestValidator, it returns http.StatusNotFound regardless of the error returned by the router.
This caused an unexpected response where the status would be status = 404, body = method not allowed when the request method didn’t match.

In this PR, I fixed validateRequest() to use errors.Is() to check the error and return the appropriate status code based on its value.

Additionally, I added test code for cases where the request method doesn't match.
This issue only occurs when combined with net/http, as both chi and gorilla probably handle this internally and return http.StatusMethodNotAllowed without needing fixes.

@nek023 nek023 requested a review from a team as a code owner October 19, 2024 15:23
@nek023 nek023 changed the title Return `http.StatusMethodNotAllowed for request method mismatch Return http.StatusMethodNotAllowed for request method mismatch Oct 19, 2024
@nek023 nek023 changed the title Return http.StatusMethodNotAllowed for request method mismatch Return http.StatusMethodNotAllowed for request method mismatch Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant